Fix behavior of useHotkey when returned ref is re-attached to a diferent element. #1117
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@vilemj-Viclick You might be interested in the const useEventCallback = <F extends (...args: any[]) => any>(fn: F) => {
const ref = useRef(fn);
useLayoutEffect(() => {
ref.current = fn;
});
return useCallback(
(...args: Parameters<F>) => (0, ref.current)(...args) as ReturnType<F>,
[]
);
};It's often used when one wants a referentially stable function that should nevertheless close over values that might change from one render to the next (and those values change frequently, or one is creating a hook that accepts a callback, since your custom hook wouldn't get a dependency array flagged by It's worth noting that once it's considered stable, using React's built-in Having said that, in this particular case, the easiest way to avoid all of the ref objects is to store the DOM instance in state, use |
|
No need for this PR anymore. A better version is proposed in #1132 . |
Motivation
This pull request fixes issue #1116.
A brief look at the code
First of all: A test!
I added a test that fails if the issue #1116 is not fixed. This allows for any other solution to be adopted, should you not like my solution.
An auxiliary type
I added a type alias for KeyboardEventHandler (of the native kind - as opposed to the React.KeyboardEventHandler). It helps make the code more concise in the hook.
The hook (useHotkeys)
The most important change here is that the returned Ref is a RefCallback. The rest of the changes are driven by this crucial step.
What the callback
The returned callback leverages refs as callbacks behavior documented here.
As stated in the next section, refCallbacks should be stable, hence the
useCallbackaround it.Why so many (use)refs?
To make the returned refCallback as stable as possible a few steps had to be made. As having any sort of
refObject.currentamong the dependencies of any hook is generally a bad practice (because it won't work as expected), we need a way to update the attached handler reliably. This is simply achieved by making the attached handler stable for the life of the component using this hook. This means we can swap the active code of the handler (handleKeyUpRef.current) without using the DOM API and we can simply add and remove the listeners (stableHandleKeyUpRef.current) based on simpler conditions or events.Other comments
The code could be further improved, by splitting the giant
useEffectinto smaller chunks and, honestly the dependencies could also be dropped with smart usage of refObjects. Updating the value of arefObject.currectwhen the user of your hook provides a new callback is virtually free. And if they want to make it stable or provide a new instance every time is their call.If we manage to get this merged into your package in this form or a refined one, I'm willing to help you further improve this package, should you be interested. 😉